Barecheck - Code coverage report

Your code coverage diff: 0.01% ▴

I think the improved format is great. I am less convinced by the code: I think this might be better handled by updating Connect to open a page to display 5xx error messages.

In the long term we should consider changing the way Connect tests for microscopes so it doesn't rely on the v1 API (unless @julianstirling did that already), and perhaps adding an explicit test for LabThings fallback servers so they can show up as broken microscopes.

I assume the dead microscope only shows up if you enter the address manually?

I was going to agree it was the wrong code until I read this

HTTP response status code 218 This is fine is sent by Apache to indicate that there is a HTTP client error or HTTP server error, but this special status code communicating success is used to maintain a standardized look and feel by the client.

It seems like it was invented exactly for this use where you want to notify there is an error but want the client to act like everything is fine and display something. I thing re-defining how electron handles 5xx errors seems like a can of worms, and if Apache felt the need for this code to exist we can't be the only ones.

Somewhat I think 5xx is the wrong error anyway, because while there is an internal server error, it isn't like it is a full 500 error which means that the server was unable to give you a helpful response. We have a helpful response with HTML content, it is just unfortunately helpful response that things are broken.

I personally would merge this now. But I won't while @rwb27 has reservations.

One option would be to make setting the code an option so that 500 can be the default given by labthings, but instead downstream can customise.

All that would be needed is (I can't suggest changes to unchanged lines in GitHub):

add to line 15:

        self.html_code = 500

as 74 would change to

    return HTMLResponse(content=content, status_code=app.html_code)

This way it becomes an OpenFlexure question of if 218 makes sense.

I have added the change to make the code stable after discussing with Joe. This way LabThings doesn't have to be opinionated about the error code.

I don't have a strong feeling for 218 vs 500 - my reservation is just that it could hide errors or confuse things, and that I have previously used HTTP codes I had just found and then decided it was non-standard. The 218 code does sound like it's exactly what we are looking for here (there's an error, but we want the client to render it like a page) so I think it is reasonable.

Julian's suggestion to move the 218 code into openflexure-microscope-server seems like a good resolution. I'm happy to merge this now.